Skip to content

Fix polylineplot snapping two points at exact same time#32953

Open
grliszas14 wants to merge 1 commit intomusescore:masterfrom
grliszas14:fix_polyline
Open

Fix polylineplot snapping two points at exact same time#32953
grliszas14 wants to merge 1 commit intomusescore:masterfrom
grliszas14:fix_polyline

Conversation

@grliszas14
Copy link
Copy Markdown
Contributor

@grliszas14 grliszas14 commented Apr 8, 2026

Resolves: audacity/audacity#10435

  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 21c3b1d6-aa88-45b1-88ba-ebe31a05ec74

📥 Commits

Reviewing files that changed from the base of the PR and between 709240a and 2343069.

📒 Files selected for processing (1)
  • src/framework/uicomponents/qml/Muse/UiComponents/polylineplot.cpp
📝 Walkthrough

Walkthrough

The polyline plot component's event handling was changed. The snapping function now offsets the snapped X by a small SNAP_EPSILON away from a neighbor instead of snapping exactly to the neighbor's X. Mouse press now emits a pointAdded with completed: false for clicks on the line and stores pending point info. Mouse release finalizes click-without-drag interactions by emitting pointAdded with completed: true for the stored point. No public or exported APIs were modified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: fixing polylineplot snapping behavior when two points reach the exact same position.
Description check ✅ Passed The PR description follows the required template, with most checklist items completed; the unit test checkbox is unchecked but noted as not applicable.
Linked Issues check ✅ Passed The code changes implement the resistance threshold mechanism required by issue #10435 to prevent immediate point merging.
Out of Scope Changes check ✅ Passed All changes in polylineplot.cpp are directly related to fixing the point snapping behavior described in the linked issue.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot reviewed Apr 8, 2026
View reviewed changes
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents 
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/framework/uicomponents/qml/Muse/UiComponents/polylineplot.cpp`:
- Around line 1364-1368: The code emits pointAdded using
m_points[m_pressedPointIndex] but only checks m_pressedPointIndex >= 0; add a
defensive upper-bound check to ensure m_pressedPointIndex < m_points.size()
before indexing. Update the mouse release / click handling branch (where
m_pressedOnLine and m_pressedPointIndex are used) to verify the index is within
range and only then read m_points and emit pointAdded (leave the existing
conditions m_pressedOnLine and isClick in place); if the index is out of range,
skip emitting to avoid out-of-bounds access.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a44f63f9-4a9c-4f20-8664-f0f24dcf42c1

📥 Commits

Reviewing files that changed from the base of the PR and between 214fea5 and 709240a.

📒 Files selected for processing (1)
  • src/framework/uicomponents/qml/Muse/UiComponents/polylineplot.cpp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@RomanPudashkin RomanPudashkin RomanPudashkin approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Gain automation: add a bit of resistance before points start eating other points

2 participants

@grliszas14 @RomanPudashkin